Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a Django Model MixIn #74

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

bernd-wechner
Copy link
Contributor

Replaces this syntax:

from django_cte import CTEManager

class Order(Model):
    objects = CTEManager()

with this:

from django_cte import CTEModelMixIn

class Order(Model, CTEModelMixIn):

entirely, optionally (that is the earlier syntax works too). This is simply a more canonical (IMHO) Django way of offering the same service (and has extensibility advantages - as in should the package ever need any more model enhancements, the MixIn is there to take them)

Replaces this syntax:

```
from django_cte import CTEManager

class Order(Model):
    objects = CTEManager()
```

with this:

```
from django_cte import CTEModelMixIn

class Order(Model, CTEModelMixIn):
```

entirely, optionally (that is the earlier syntax works too). This is simply a more canonical (IMHO) Django way of offering the same service (and has extensibility advantages - as in should the package ever need any more model enhancements, the MixIn is there to take them)
Copy link
Contributor

@millerdev millerdev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this adds enough value to be worth it at this point. "There should be one-- and preferably only one --obvious way to do it."

@bernd-wechner
Copy link
Contributor Author

If there was only one way to do it, I'd still vote for a MixIn rather than overriding an attribute, as it strikes me as both more Django and is more extensible. Either way, it's a small value add.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants